[#284] Fix false recovery intents from wallet rejection#286
[#284] Fix false recovery intents from wallet rejection#286realproject7 merged 1 commit intomainfrom
Conversation
Move intent save to after writeContractAsync succeeds instead of before. If the wallet rejects (any error message, any wallet), no intent is ever written to localStorage. Removes brittle isUserRejection() pattern matching entirely. Fixes #284 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
T2b Review — APPROVED
Elegant fix. Moving onIntentSave + onTxConfirmed to after writeContractAsync succeeds means:
- Wallet rejection = no intent ever written (no false recovery)
isUserRejection()heuristic eliminated entirely (no string-matching fragility)- The semantic
onIndexedmisuse from PR #282 is gone as a side effect
Intent + txHash are saved back-to-back after wallet signs but before receipt polling — if the page crashes between sign and receipt confirmation, recovery still works correctly. Net -20 lines, strictly better.
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
This change removes the brittle wallet-rejection string matching and fixes the false recovery-intent bug by only writing intent state after writeContractAsync succeeds. The earlier receipt-poll and indexer recovery paths remain intact because the tx hash is still persisted before receipt waiting.
Findings
- No blocking findings from this review pass.
Decision
Approving from my side. GitHub checks were still pending at the time of review.
2 participants
Summary
onIntentSave+onTxConfirmedto afterwriteContractAsyncsucceeds, not beforeisUserRejection()entirely — no more brittle string matching against wallet error messagesRoot cause
The old flow saved intent before the wallet popup. If the wallet rejected with a non-standard error message (e.g., Rabby, Frame, hardware wallets),
isUserRejection()wouldn't match, leaving a false intent that triggered the recovery banner for a tx that never happened.Test plan
npm run typecheckpassesFixes #284
🤖 Generated with Claude Code